Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: add an async dhcp client #1250

Draft
wants to merge 17 commits into
base: master
Choose a base branch
from
Draft

Conversation

etene
Copy link
Contributor

@etene etene commented Jan 14, 2025

Here is my async DHCP client implementation, still a work in progress

closes #1243

CLIENT_SYSTEM_ARCHITECTURE_TYPE = 93 # Client System Architecture Type
CLIENT_NETWORK_INTERFACE_IDENTIFIER = 94 # Client Network Interface Identifier
CLASSLESS_STATIC_ROUTE = 121
DOMAIN_SEARCH = 119
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one is not sorted?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i added it because at some point it was missing when i was testing something, but this is still a draft, these are not even used at this point

@svinota
Copy link
Owner

svinota commented Jan 14, 2025

Wow, c'est magnifique. It will take some time to read & digest, be patient :)

@etene
Copy link
Contributor Author

etene commented Jan 14, 2025

Wow, c'est magnifique. It will take some time to read & digest, be patient :)

Thanks ! Take your time, it's still a draft and lots of things are going to change, i'm not yet proud with everything

@etene etene marked this pull request as draft January 14, 2025 20:53
@etene etene force-pushed the async-dhcp-client branch 4 times, most recently from 12c5eb9 to 14cdae0 Compare January 15, 2025 08:40
# bring up the socket
socket.__init__(self, AF_PACKET, SOCK_RAW, htons(ETH_P_ALL))
socket.setblocking(self, False)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@etene what would you say — could it be a reasonable idea to make a socket wrapper that would use composition, and proxy most of the properties, but fake .type to make it work with loop.create_datagram_endpoint() ? and then use asyncio.Protocol

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's an interesting idea.

I have tried at some point to make raw sockets work with asyncio.Protocol but changed my mind when i saw how a lot of code assumed TCP, UDP or Unix sockets iirc. But I hadn't thought about faking the socket type, and it might just work given how similar raw & UDP sockets are.

I'm willing to try if I find the time to. However, the python stdlib code might be too tightly coupled with UDP/TCP/unix sockets, so it might turn out to be dirty or impossible without making a PR to python itself, which is a whole other can of worms...

@etene etene force-pushed the async-dhcp-client branch from 14cdae0 to 353efde Compare January 15, 2025 08:45
@svinota
Copy link
Owner

svinota commented Jan 15, 2025

@etene JFYI: I plan to keep Python compatibility back to version 3.9 as for now. Will be there any issues with that — pls let me know.

@etene
Copy link
Contributor Author

etene commented Jan 15, 2025

@etene JFYI: I plan to keep Python compatibility back to version 3.9 as for now. Will be there any issues with that — pls let me know.

@svinota I'm pretty sure i can make it work with 3.9 upwards, my initial draft was based on 3.12 out of habit and comfort but the really important asyncio parts have been there since 3.7.

@svinota
Copy link
Owner

svinota commented Jan 15, 2025

The reason behind 3.9 (FIXME: I should document it) is socket.recv_fds() that is not supported in earlier versions — and which is used in the core functionality to init netns-enabled sockets.

@etene etene force-pushed the async-dhcp-client branch from f9376db to e0566b0 Compare January 16, 2025 16:51
@etene etene force-pushed the async-dhcp-client branch from 31c1b42 to d6fa1e5 Compare January 17, 2025 19:06
@etene etene force-pushed the async-dhcp-client branch from d6fa1e5 to 417a8d9 Compare January 17, 2025 19:13
@svinota
Copy link
Owner

svinota commented Jan 18, 2025

@etene regarding the tests I would propose this thing:

  1. save binary packets (optionally, store them as hex dumps, see [1])
  2. use config.mock_dhcp in a way like [2][3]
  3. having this, just mock the traffic through the socket, and assert every state transition
  4. bonus: now code snippets in the docs can be tested as well, to keep the docs up to date automatically, like in [4][5]

[1]

from pyroute2.common import load_dump
test_dump_data = '''
# 10.1.2.0/24 via 127.0.0.8 dev lo table 100
# 10.1.3.0/24 via 127.0.0.8 dev lo table 100
24:12:31:45 3c:00:00:00 18:00:22:00 dc:d0:86:67
7b:68:00:00 02:18:00:00 64:03:00:01 00:00:00:00
08:00:0f:00 64:00:00:00 08:00:01:00 0a:01:02:00
08:00:05:00 7f:00:00:08 08:00:04:00 01:00:00:00
3c:00:00:00 18:00:22:00 dc:d0:86:67 7b:68:00:00
02:18:00:00 64:03:00:01 00:00:00:00 08:00:0f:00
64:00:00:00 08:00:01:00 0a:01:03:00 08:00:05:00
7f:00:00:08 08:00:04:00 01:00:00:00
'''

[2]
if config.mock_iproute:
use_socket = IPEngine()
netns = None

[3]
.. testsetup:: *
from pyroute2 import config, IPRoute
config.mock_netlink = True
ipr = IPRoute()
.. testcleanup:: *
ipr.close()

[4] source:
.. testcode:: fm01
# get all links with names starting with eth:
#
for link in ipr.filter_messages(
lambda x: x.get("ifname").startswith("eth"),
ipr.link("dump"),
):
print(link.get("ifname"))
.. testoutput:: fm01
eth0

[5] rendered: https://pyroute2.org/pyroute2-0.8.1-109-g19a309be/iproute_linux.html#pyroute2.iproute.linux.RTNL_API.filter_messages

def get_cmdline_options(self) -> tuple[str]:
'''All commandline options passed to udhcpd.'''
return (
'udhcpd',
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use busybox udhcpd instead, on Fedora there is no separate link for that, unlike Ubuntu; but running the plugin this way as busybox udhcpd works on both Fedora and Ubuntu.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DHCP client
3 participants